Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feature/dynamodb/attributevalue: remove redundent tryMarshaler calls in Encoder.encodeString and Encoder.encodeNumber #2570

Merged
merged 2 commits into from
Mar 25, 2024
Merged

feature/dynamodb/attributevalue: remove redundent tryMarshaler calls in Encoder.encodeString and Encoder.encodeNumber #2570

merged 2 commits into from
Mar 25, 2024

Conversation

OrHayat
Copy link
Contributor

@OrHayat OrHayat commented Mar 20, 2024 •
edited
Loading

both Encoder.encodeString and Encoder.encodeNumber get called ONLY
from main encoder.encode function and they always get called after tryMarshaller was already attempted and failed
to do custom marshal of the struct
that cause many many redundant calls into reflect package which is quite slow and this code can be removed safely

  • added tests to show that the code removal is ok
  • added benchmark for marshaling list of 20 ints and struct of 10 fields with string/int members both cases are quite reasonable use-case for dynamodb and will show that the code got faster

ran the benchmarks before and after the changes on my pc and you can see that the performance of marshal improved by quit a lot (result vary alot but on average the result of go bench are better after the change)

before:
 > ./attributevalue.test  -test.benchtime 10s -test.benchmem -test.run=^$ -test.bench . github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue
goos: linux
goarch: amd64
pkg: github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue
cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
BenchmarkMarshalOneMember-8              7318168              2292 ns/op            1641 B/op         25 allocs/op
BenchmarkList20Ints-8                    4120447              3451 ns/op            1080 B/op         45 allocs/op
BenchmarkStruct10Fields-8                5423558              2392 ns/op            1366 B/op         17 allocs/op
BenchmarkMarshalTwoMembers-8             2497726              4516 ns/op            2866 B/op         44 allocs/op
BenchmarkUnmarshalOneMember-8           14122314               852.7 ns/op           112 B/op          3 allocs/op
BenchmarkUnmarshalTwoMembers-8           7986501              1752 ns/op             112 B/op          3 allocs/op
PASS

after:
./attributevalue.test  -test.benchtime 10s -test.benchmem -test.run=^$ -test.bench . github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue
goos: linux
goarch: amd64
pkg: github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue
cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
BenchmarkMarshalOneMember-8              5826633              2183 ns/op            1641 B/op         25 allocs/op
BenchmarkList20Ints-8                    4771832              2237 ns/op            1080 B/op         45 allocs/op
BenchmarkStruct10Fields-8                6825120              1999 ns/op            1366 B/op         17 allocs/op
BenchmarkMarshalTwoMembers-8             2751194              4074 ns/op            2866 B/op         44 allocs/op
BenchmarkUnmarshalOneMember-8           16283137               752.3 ns/op           112 B/op          3 allocs/op
BenchmarkUnmarshalTwoMembers-8           8588299              1324 ns/op             112 B/op          3 allocs/op
PASS

@OrHayat OrHayat requested a review from a team as a code owner March 20, 2024 15:01
@OrHayat OrHayat changed the title remove redundent tryMarshaler calls in encodeString and encodeNumber feature/dynamodb/attributevalue: remove redundent tryMarshaler calls in encodeString and encodeNumber Mar 20, 2024
@OrHayat OrHayat changed the title feature/dynamodb/attributevalue: remove redundent tryMarshaler calls in encodeString and encodeNumber feature/dynamodb/attributevalue: remove redundent tryMarshaler calls in Encoder.encodeString and Encoder.encodeNumber Mar 20, 2024
remove redundent tryMarshaler calls in encodeString and encodeNumber
dc0a215 
both encodeString and encodeNumber get called from encode after tryMarshaller failled

added benchmark for list of int and struct of int and string fields
@OrHayat OrHayat force-pushed the remove_redundent_try_marshal_calls branch from 2aa06b0 to dc0a215 Compare March 20, 2024 20:17
lucix-aws
lucix-aws approved these changes Mar 22, 2024
View reviewed changes
Copy link
Contributor

@lucix-aws lucix-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid find

@lucix-aws lucix-aws merged commit 7354cd9 into aws:main Mar 25, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucix-aws lucix-aws lucix-aws approved these changes

@wty-Bryant wty-Bryant wty-Bryant approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@OrHayat @wty-Bryant @lucix-aws